Skip to content

New sniff to prefer accessing properties from FrmAppHelper::get_settings directly#2889

Merged
Crabcyborg merged 3 commits into
masterfrom
new_sniff_to_prefer_accessing_properties_from_frmapphelper_get_settings_directly
Jan 27, 2026
Merged

New sniff to prefer accessing properties from FrmAppHelper::get_settings directly#2889
Crabcyborg merged 3 commits into
masterfrom
new_sniff_to_prefer_accessing_properties_from_frmapphelper_get_settings_directly

Conversation

@Crabcyborg

@Crabcyborg Crabcyborg commented Jan 27, 2026

Copy link
Copy Markdown
Contributor

This helps remove an extra line of code in a few places, and a reference to an extra variable that we don't need to be setting first.

Summary by CodeRabbit

  • Refactor

    • Simplified internal code structure for improved maintainability and reduced complexity.
  • Chores

    • Updated property annotations for type clarity.
    • Enhanced code analysis tooling to detect and suggest optimizations for settings retrieval patterns.
    • Updated code sniff documentation examples.

✏️ Tip: You can customize this high-level summary in your review settings.

@Crabcyborg Crabcyborg added this to the 6.28 milestone Jan 27, 2026
@Crabcyborg Crabcyborg changed the title New sniff to prefer accessing properties from frmapphelper get settings directly New sniff to prefer accessing properties from FrmAppHelper::get_settings directly Jan 27, 2026
@coderabbitai

coderabbitai Bot commented Jan 27, 2026

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

This pull request refactors three codebase locations to inline FrmAppHelper::get_settings() calls, eliminating intermediate variables. It introduces a new PHP CodeSniffer sniff to automatically detect and fix this pattern, updates type annotations in FrmSettings to reflect actual return types, and corrects a code example in an existing sniff.

Changes

Cohort / File(s) Summary
Inline Settings Refactoring
classes/controllers/FrmUsageController.php, classes/models/FrmHoneypot.php
Simplified methods by removing intermediate $settings/$frm_settings variables and inlining FrmAppHelper::get_settings() calls with explicit boolean casts. Control flow unchanged.
Type Annotation Updates
classes/models/FrmSettings.php
Updated docblock annotations: $tracking changed from bool to bool|int, and $honeypot changed from string to int to reflect actual property types.
New Code Analysis Sniff
phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/InlineFrmSettingsReturnSniff.php
Introduces a new sniff that detects assignments of FrmAppHelper::get_settings() followed by immediate variable usage in return statements, suggesting inlining. Includes pattern matching logic and token replacement utilities.
Sniff Ruleset Configuration
phpcs-sniffs/Formidable/ruleset.xml
Registered the new InlineFrmSettingsReturnSniff rule in the Code Analysis section.
Example Code Comment Update
phpcs-sniffs/Formidable/Sniffs/CodeAnalysis/FlipNegativeTernarySniff.php
Updated example patterns in comments from null !== $var to 'value' !== $var for clarity. No functional changes to sniff logic.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Poem

🐰 Hops of joy for cleaner code,
No more variables on the road,
Direct the flow, let settings inline,
A sniff to guide them—oh so fine!
Refactored patterns, clear and bright,
CodeSniffer hops through files at night. 🌙✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the primary change: introducing a new sniff to inline FrmAppHelper::get_settings() property access, which is implemented across multiple files and is the main feature of this PR.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@Crabcyborg
Crabcyborg merged commit 633e301 into master Jan 27, 2026
15 of 16 checks passed
@Crabcyborg
Crabcyborg deleted the new_sniff_to_prefer_accessing_properties_from_frmapphelper_get_settings_directly branch January 27, 2026 13:53
stephywells pushed a commit that referenced this pull request Apr 4, 2026
…g_properties_from_frmapphelper_get_settings_directly

New sniff to prefer accessing properties from `FrmAppHelper::get_settings` directly
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant